Document stack-protector option #111722

mrcnski · 2023-05-18T12:52:52Z

Only updated exploit-mitigations.md to reflect that the option exists. Removed the alternatives mentioned as they are not actually implemented yet.

As this is an unstable feature, should it be added to unstable-book also? Example. I didn't do that because I couldn't find the tracking issue for stack-protector. (There should be one to track stabilization of the feature, I think?)

cc @rcvalle

Only updated `exploit-mitigations.md` to reflect that the option exists. Removed the alternatives mentioned as they are not actually implemented yet. As this is an unstable feature, should it be added to `unstable-book` also? I didn't do that because I couldn't find the tracking issue for it. (There should be one to track stabilization of the feature.)

rustbot · 2023-05-18T12:52:59Z

r? @JohnTitor

(rustbot has picked a reviewer for you, use r? to override)

rcvalle · 2023-05-19T05:26:30Z

Here is an updated "Fig. 14. IDA Pro listing cross references to __stack_chk_fail in hello-rust." (i.e., image3.png) for you to update:

rcvalle · 2023-05-19T05:33:07Z

I also suggest changing

To check if stack smashing protection is enabled for a given binary, search
for cross references to `__stack_chk_fail`. The only cross references to
`__stack_chk_fail` in hello-rust are from the statically-linked libbacktrace
library (see Fig. 14).

to

To check if stack smashing protection is enabled for a given binary, search
for cross references to `__stack_chk_fail` (see Fig. 14).

JohnTitor · 2023-05-19T06:02:21Z

r? rcvalle

mrcnski · 2023-05-19T21:16:28Z

Thanks for the review @rcvalle! Will make the changes, I just want to make sure I understand them correctly: does the presence of cross-references to __stack_chk_fail in the binary mean that stack-protector is turned on? If so, is it expected that the number of cross-references went down since the feature was implemented? (15 -> 1)

rcvalle · 2023-05-19T23:03:20Z

Thanks for the review @rcvalle! Will make the changes, I just want to make sure I understand them correctly: does the presence of cross-references to __stack_chk_fail in the binary mean that stack-protector is turned on?

Yes, the presence of cross-references to __stack_chk_fail from Rust-compiled code (e.g., hello_rust::main) indicates that the stack smashing protection is enabled.

If so, is it expected that the number of cross-references went down since the feature was implemented? (15 -> 1)
The only cross references to

Note that previously there were no cross-references to __stack_chk_fail from Rust-compiled code. The only cross references to __stack_chk_fail were from the statically-linked libbacktrace library (i.e., non Rust-compiled code).

mrcnski · 2023-05-21T12:23:46Z

Done!

rcvalle · 2023-05-22T17:37:59Z

Thank you for your time and for working on this, @mrcnski! Much appreciated.

rcvalle · 2023-05-22T17:38:10Z

@bors r+

bors · 2023-05-22T17:38:13Z

@rcvalle: 🔑 Insufficient privileges: Not in reviewers

cuviper · 2023-05-22T17:42:26Z

@bors r=rcvalle

bors · 2023-05-22T17:42:28Z

📌 Commit a4d6d9a has been approved by rcvalle

It is now in the queue for this repository.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#111427 ([rustdoc][JSON] Use exclusively externally tagged enums in the JSON representation) - rust-lang#111486 (Pretty-print inherent projections correctly) - rust-lang#111722 (Document stack-protector option) - rust-lang#111761 (fix(resolve): not defined `extern crate shadow_name`) - rust-lang#111845 (Update books) - rust-lang#111851 (CFI: Fix encode_region: unexpected ReEarlyBound(0, 'a)) - rust-lang#111871 (Migrate GUI colors test to original CSS color format) r? `@ghost` `@rustbot` modify labels: rollup

@bbjornse

I propose stabilizing `-Cstack-protector` as `-Zstack-protector`. This PR adds a new `-Cstack-protector` flag, leaving the unstable `-Z` flag as is to ease the transition period. The `-Z` flag will be removed in the future. No RFC/MCP, this flag was added in rust-lang#84197 and was not deemed large enough to require additional process. The tracking issue for this feature is rust-lang#114903. The `-Cstack-protector=strong` mode uses the same underlying heuristics as Clang's `-fstack-protector-strong`. These heuristics weren't designed for Rust, and may be over-conservative in some cases - for example, if Rust stores a field's data in an alloca using an LLVM array type, LLVM regard the alloca as meaning that the function has a C array, and enable stack overflow canaries even if the function accesses the alloca in a safe way. Some people thought we should wait on stabilization until there are better heuristics, but I didn't hear about any concrete case where this unduly harms performance, and I think that when a need comes, we can improve the heuristics in LLVM after stabilization. The heuristics do seem to not be under-conservative, so this should not be a security risk. The `-Cstack-protector=basic` mode (`-fstack-protector`) uses heuristics that are specifically designed to catch old-C-style string manipulation. This is not a good fit to Rust, which does not perform much unsafe C-style string manipulation. As far as I can tell, nobody has been asking for it, and few people are using it even in today's C - modern distros (e.g. [Debian]) tend to use `-fstack-protector-strong`. Therefore, `-Cstack-protector=basic` has been **removed**. If anyone is interested in it, they are welcome to add it back as an unstable option. [Debian]: https://wiki.debian.org/Hardening#DEB_BUILD_HARDENING_STACKPROTECTOR_.28gcc.2Fg.2B-.2B-_-fstack-protector-strong.29 Most implementation was done in <rust-lang#84197>. The command-line attribute enables the relevant LLVM attribute on all functions in <https://github.com/rust-lang/rust/blob/68baa87ba6f03f8b6af2a368690161f1601e4040/compiler/rustc_codegen_llvm/src/attributes.rs#L267-L276>. Each target can indicate that it does not support stack canaries - currently, the GPU platforms `nvptx64-nvidia-cuda` and `amdgcn-amd-amdhsa`. On these platforms, use of `-Cstack-protector` causes an error. The feature has tests that make sure that the LLVM heuristic gives reasonable results for several functions, by checking for `__security_check_cookie` (on Windows) or `__stack_chk_fail` (on Linux). See <https://github.com/rust-lang/rust/tree/68baa87ba6f03f8b6af2a368690161f1601e4040/tests/assembly-llvm/stack-protector> No call-for-testing has been conducted, but the feature seems to be in use. No reported bugs seem to exist. - @bbjornse was the original implementor at rust-lang#84197 - @mrcnski documented it at rust-lang#111722 - @wesleywiser added tests for Windows at rust-lang#116037 - @davidtwco worked on the feature at rust-lang#121742 - @nikic provided support from the LLVM side (on Zulip on <https://rust-lang.zulipchat.com/#narrow/channel/233931-t-compiler.2Fmajor-changes/topic/Proposal.20for.20Adapt.20Stack.20Protector.20for.20Ru.E2.80.A6.20compiler-team.23841> and elsewhere), thanks @nikic! No FIXMEs related to this feature. This feature cannot cause undefined behavior. No changes to reference/spec, docs added to the codegen docs as part of the stabilization PR. No. None. No support needed for rustdoc, clippy, rust-analyzer, rustfmt or rustup. Cargo could expose this as an option in build profiles but I would expect the decision as to what version should be used would be made for the entire crate graph at build time rather than by individual package authors. `-C stack-protector` is propagated to C compilers using cc-rs via rust-lang/cc-rs#1550

rustbot assigned JohnTitor May 18, 2023

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 18, 2023

rustbot assigned rcvalle and unassigned JohnTitor May 19, 2023

rcvalle added the PG-exploit-mitigations Project group: Exploit mitigations label May 19, 2023

rcvalle linked an issue May 19, 2023 that may be closed by this pull request

Document stack smashing protection #93052

Closed

doc: Update exploit-mitigations.md, update image3.png

a4d6d9a

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 22, 2023

matthiaskrgr mentioned this pull request May 23, 2023

Rollup of 7 pull requests #111882

Merged

bors merged commit ee08dd8 into rust-lang:master May 23, 2023

rustbot added this to the 1.71.0 milestone May 23, 2023

rcvalle mentioned this pull request Aug 16, 2023

Tracking Issue for stabilizing stack smashing protection (i.e., -Z stack-protector) #114903

Open

davidtwco mentioned this pull request Feb 28, 2024

sess: stabilize -C stack-protector=all #121742

Closed

arielb1 mentioned this pull request Sep 9, 2025

Stabilize stack protector #146369

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Document stack-protector option #111722

Document stack-protector option #111722

Uh oh!

mrcnski commented May 18, 2023

Uh oh!

rustbot commented May 18, 2023

Uh oh!

rcvalle commented May 19, 2023

Uh oh!

rcvalle commented May 19, 2023

Uh oh!

JohnTitor commented May 19, 2023

Uh oh!

mrcnski commented May 19, 2023 •

edited

Loading

Uh oh!

rcvalle commented May 19, 2023 •

edited

Loading

Uh oh!

mrcnski commented May 21, 2023

Uh oh!

rcvalle commented May 22, 2023

Uh oh!

rcvalle commented May 22, 2023

Uh oh!

bors commented May 22, 2023

Uh oh!

cuviper commented May 22, 2023

Uh oh!

bors commented May 22, 2023

Uh oh!

Uh oh!

Document stack-protector option #111722

Document stack-protector option #111722

Uh oh!

Conversation

mrcnski commented May 18, 2023

Uh oh!

rustbot commented May 18, 2023

Uh oh!

rcvalle commented May 19, 2023

Uh oh!

rcvalle commented May 19, 2023

Uh oh!

JohnTitor commented May 19, 2023

Uh oh!

mrcnski commented May 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rcvalle commented May 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrcnski commented May 21, 2023

Uh oh!

rcvalle commented May 22, 2023

Uh oh!

rcvalle commented May 22, 2023

Uh oh!

bors commented May 22, 2023

Uh oh!

cuviper commented May 22, 2023

Uh oh!

bors commented May 22, 2023

Uh oh!

Uh oh!

mrcnski commented May 19, 2023 •

edited

Loading

rcvalle commented May 19, 2023 •

edited

Loading